Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Generate LODs, Shadow Mesh and Lightmap UV2 options to OBJ mesh import #94108

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Jul 8, 2024

This puts OBJ mesh import on parity with 3D scene import. It's now possible to have the same level of optimization as imported 3D scenes while using the OBJ mesh workflow.

LOD and shadow mesh generation are enabled by default, so that users can benefit from the same performance regardless of how they're importing 3D objects into Godot. Lightmap UV2 generation is disabled by default to mimic the 3D scene import default (and PrimitiveMesh).

Testing project: test_obj_import_lod_shadow_mesh_lightmap_uv2.zip

Preview

OBJ mesh import options

image

Lightmapped OBJ mesh having its LOD level changed

obj_lod_lightmap.mp4

LOD disabled

Screenshot_20240709_002424 png webp

LOD enabled (Mesh LOD Threshold = 1024)

This shows the most decimated LODs for comparison purposes.

Screenshot_20240709_002431 png webp

TODO

  • Figure out why it runs much slower than 3D scene import when LOD generation or lightmap UV2 generation is enabled. (Shadow mesh generation also suffers from this problem to a less severe extent.)
    • This does not occur with 3D scene import, even on the same OBJ file switched to use the OBJ as Scene import mode.

This option was enabled by default, but it did nothing regardless
of whether it was enabled or not.
@Calinou Calinou added this to the 4.x milestone Jul 8, 2024
@Calinou Calinou requested review from a team as code owners July 8, 2024 23:16
@Calinou Calinou changed the title Remove unused Optimize Mesh import option from OBJ mesh import Add Generate LODs, Shadow Mesh and Lightmap UV2 options to OBJ mesh import Jul 8, 2024
…mport

This puts OBJ mesh import on parity with 3D scene import. It's now
possible to have the same level of optimization as imported 3D scenes
while using the OBJ mesh workflow.
@Calinou Calinou force-pushed the obj-import-add-generate-lods-shadow-mesh-lightmap-uv2 branch from 3ca15cd to 60c20bf Compare July 8, 2024 23:25
Comment on lines +475 to +482
if (p_generate_lods) {
// Use normal merge/split angles that match the defaults used for 3D scene importing.
mesh->generate_lods(60.0f, 25.0f, {});
}

if (p_generate_shadow_mesh) {
mesh->create_shadow_mesh();
}
Copy link
Member Author

@Calinou Calinou Jul 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: if you perform these steps before lightmap UV2 generation, the resulting mesh will be broken. It'll display with the wrong transform and no material (pure black color). I have no idea why this happens, but I figure I'd mention it either way. Putting up a MRP for this will likely require exposing additional ImporterMesh methods (such as create_shadow_mesh() and lightmap_unwrap_cached()).

@fracteed
Copy link

fracteed commented Jul 9, 2024

Nice work. I would look to see an option to not to try to import .mtl files when importing an obj. It is a constant annoyance to see errors in the console that it can't find the corresponding .mat file for an obj.

Personally, I never use the .mtl file, so only export it to stop Godot spitting out the error. Godot will see face groups fine with just the .obj itself.

@lyuma
Copy link
Contributor

lyuma commented Jul 9, 2024

I would look to see an option to not to try to import .mat files

@fracteed To be clear, you're talking about .mtl files referenced by a mtllib statement in an obj

My opinion is that it is data which is directly referenced by the obj file, so it would be wrong not to try to load it.

I don't think we should add an option to disable an error, especially when there are cases where the error is helpful. If you really don't have the .mtl, it's also relatively easy to edit the .obj file and remove the mtllib statement.

@fracteed
Copy link

fracteed commented Jul 9, 2024

@lyuma yeah, I meant to write mtl. There should at least be an option to turn off the requirement for it for those that just need the mesh information. If I have hundreds of objs in a project, it is not viable to fiddle with editing obj files.

Just went and checked on a current build to see if anything had changed, as I probably wasn't very clear with my initial comment. Yes, you can export objs without an mtl and they import fine without any error messages. But the issue is that you can't get material group selections unless you have the mtl file exported from Blender.

It is a stupid quirk in the format, since if I export an obj from blender with the mtl and then delete the mtl, Godot will still import it with the material groups intact, it will however give an error message. One of the main reasons I delete the mtl is that Godot deals with them badly in general. For one, they do not show up in the file system, and secondly, when you move an obj from one folder to another, it does not move the mtl file along with the obj.

So after having hundreds of files added and deleted over the course of years in a large project you end up with a ton of errors looking for mtl files that are actually not needed, and are either in other folders or long deleted. I guess the ideal would be if Godot could actually just read the material group info directly from the obj, since the face info is in there, as you can can export an obj with material groups and no mtl.

@Calinou
Copy link
Member Author

Calinou commented Jul 9, 2024

It is a constant annoyance to see errors in the console that it can't find the corresponding .mat file for an obj.

#94102 fixes the spurious error message if you have no MTL file referenced in the OBJ. However, I think a missing MTL file should print a message if the OBJ is trying to reference it, as it can be the source of a legitimate error.

If I have hundreds of objs in a project, it is not viable to fiddle with editing obj files.

sd should be able to automate this easily.

Something like this can be used to comment out all material file definitions in OBJ files in the current folder (recursively):

# Use this to make `**` work on Bash (not required on zsh).
shopt -s globstar nullglob

sd "mtllib" "#mtllib" **/*.obj

@akien-mga
Copy link
Member

I think we should add an import option for OBJ to ignore MTL files (i.e. just skip mtllib references). IIRC Godot doesn't really support the old specular mapping they use, so only bits of it can be imported. More often than not people might want to write their own material and not use the one a thirdparty OBJ file may have come with.

@zeux
Copy link
Contributor

zeux commented Jul 12, 2024

I think the extra processing steps should be done in import_scene or at the end of parse obj, not inside the loop? An .obj file with multiple materials or surface groups will add multiple surfaces to the same mesh, but with this change shadow mesh or LOD or lightmap generation will be called on the entire mesh for every newly added surface. For lightmaps this might be mitigated with a cache, but for LOD/shadow meshes it looks like the code will redundantly reprocess all earlier computed surfaces.

@zeux
Copy link
Contributor

zeux commented Jul 12, 2024

Also after #94241 it would make sense to call ImporterMesh::optimize_indices_for_cache as well; the improvements in create_shadow_mesh will automatically apply here with no changes.

@clayjohn
Copy link
Member

Since #94241 has been merged. Could you add ImporterMesh::optimize_indices_for_cache as part of this PR? I would love to include this in 4.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants